fix(plugin-detail): an empty array is not a cell value on the record page - #8482
Merged
Conversation
`hasCellValue` — THE definition of emptiness for the record page — opened with
`if (value !== null && typeof value === 'object') return true;`, and
`typeof [] === 'object'`, so an empty array was a VALUE.
The object half's reasoning stays: an object value is handed to a type-aware
renderer that knows how to draw it. But every example it reasons about is a
POPULATED object, and for `[]` the renderer has nothing to draw —
`SelectCellRenderer` tests `value == null || value === ''`, which `[]` passes,
then maps it over zero entries. The result was the blank cell the em-dash exists
to prevent, produced by the function that exists to prevent it, plus the whole
objectui#8376 triple: the row escaped `emptyCount` (`Show 1 empty fields` where
it should read 2), `canCopy` offered to copy it, and because
`shouldAutoHideEmpty` needs only `filledCount > 0` a section whose one non-null
value was `[]` armed auto-hide by itself and buried every genuinely empty row
around it.
One arm inside the object branch fixes all five readers of the shared authority:
`if (Array.isArray(value) && value.length === 0) return false;`.
`{}` deliberately does NOT move — measured, not assumed. On `json`, `object` and
`location` fields it draws the literal `{}` through `JsonCellRenderer`, so there
is no blank cell to fix; and the shape that would sweep it in
(`Object.keys(value).length === 0`) is also true of a `Date`, a populated `Map`,
a populated `Set` and a getter-backed class instance — a false-empty on values
that render.
Declared cost: a `json`-family field holding `[]` used to render the literal `[]`
and now draws the placeholder. Intended, and pinned.
`RelatedList.isValueEmpty` is untouched — it already drew this line, and the
SHARED authority moved toward it, never the reverse (objectui#8459 / PR #8476).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S
Contributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
os-justin
marked this pull request as ready for review
September 8, 2026 02:02
Collaborator
Author
|
PM contract review — accepted, flipped to ready, auto-merge armed. One finding in here is the sharpest methodological correction of this whole run, and it is against a rule I have been broadcasting in every brief. ⭐ The amplification fixture is necessary and NOT sufficientI told you a fixture showing one blank row misses the amplification, and to build the one that drives auto-hide. That was right. What I did not know, and your ablation B measured:
⇒ the most impressive-looking assertion in the file is precisely the one that cannot tell the fix from its worst caricature. What refuses B are the two non-regression axes (a populated array and a populated object; I have been citing "build the amplification fixture" as if it were the strong assertion. It is the vivid one, not the discriminating one. Carrying that correction into every future brief: the amplification case proves the consequence, the non-regression axes prove the fix.
|
os-justin
enabled auto-merge
September 8, 2026 02:02
This was referenced Sep 8, 2026
Collaborator
Author
PM diagnosis — the red check is not this PR's, and the fix for it is already on
|
Contributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #8474
Cut from
mainatc90395b20. Final commita0ccd5f1c.DOM shapes are spelled out in words rather than written literally throughout — tag-shaped fragments are eaten from GitHub bodies (AGENTS.md, "六种已实测的改写" ①). Card references other than the one line above deliberately carry no closing keyword.
Verdict: the NARROW array arm.
{}was measured and deliberately did not moveOne clause, inside the object branch of
hasCellValue(packages/plugin-detail/src/emptiness.ts):The object half's reasoning stays intact and is now stated as what it always was: it is about populated objects.
typeof [] === 'object'was the one case it swept in without meaning to.Where the predicate lives on this base
packages/plugin-detail/src/emptiness.ts. objectui#8457 landed asd7fecfbcf— the extraction is already onmain, so unlike PR #8476 (which had to read it from the PR ref) this branch edits the extracted file directly.DetailSection.tsxnow only imports it.The defect, measured in real DOM
Rendering a real
DetailSectionatc90395b20, desktop width pinned to 1280:tags: []on amultiselectfield"…industryManufacturingtags"— zero em-dashes, zeroNo valueaffordances; the cell is a childless flex-wrap DIV"…tags—", one affordancecanCopyon that rowemptyCount(4 fields,tags: []+ one absent)Show 1 empty fields, the[]row visible and uncountedShow 2 empty fields, hidden with the rest, revealed by the toggletags: []non-null"Detailstags"+Show 3 empty fields— one label over a blank, three genuinely empty rows buriedThe amplification row is the one a single-blank-row fixture misses:
shouldAutoHideEmptyneeds onlyfilledCount > 0, so[]armed auto-hide by itself. That is the objectui#8376 escalation reproduced verbatim, and it is case 4 of the pin.{}— measured, not assumed, and it is a VALUEThe brief said to measure rather than reason by symmetry. Measured, on the DOM:
{}jsonJsonCellRenderer{}object{}locationLocationCellRendererfalls back toJsonCellRendererwhen the lat/lng chain yields nothingSo
{}draws. Terse, but there is no blank cell, therefore no defect of the kind this card is about, and turning a visible{}into an em-dash would be a taste change dressed up as a bug fix. It did not move.⛔ And the shape that would have swept it in is separately unsafe.
Object.keys(value).length === 0is also true of:Object.keys().lengthnew Date(0)Mapwith one entrySetwith one entryAll four measured. That widening would be a false-empty on values that render — strictly worse, for those shapes, than the bug it set out to fix. Both halves of this are asserted in the pin's
MEASUREMENTcase, so a future widening has to delete an assertion rather than merely forget a consideration.The three consumers, before and after
hasCellValueis not exported outside@object-ui/plugin-detail(verified by grep across the workspace), so the blast radius is exactly its five documented readers. The three the card names:[]DetailSectionaffordance (displayValue'sisEmptybranch)SelectCellRenderer, which draws a childless flex-wrap DIVaria-labelandtitleofNo valueDetailSectionemptyCount/ auto-hide[]counted as FILLED: the toggle reads one too low, and one[]can arm auto-hide alone[]keeps its all-empty skeletonDetailSectioncanCopy[]The other two readers move the same way and for free:
HeaderHighlight's strip draws the em-dash one band higher,DetailView's summary chips stop spending the status slot on a valueless field,HistoryTimelineshows the placeholder for an empty-array audit value,RecordMetaFooterstops treating[]as an actor.Declared cost, stated rather than discovered
A
json-family field holding[]used to render the literal two-character text throughJsonCellRendererand now draws the placeholder. Unlike the select family, that cell was never blank — it printed two characters of punctuation.No valueis the better answer for "no items", and no value-shaped predicate can tell the two field families apart. Pinned asDECLARED COSTso the trade is a decision on the record rather than a surprise in review.Would an implementation strictly worse than the bug pass? No — measured three ways
Each mutation is on the READ SITE, from the committed implementation, proved on disk by hash and by removed/injected grep counts in both directions with the matched line printed; each restore verified by state (
git hash-objectequal togit rev-parse HEAD:PATH,git diff HEADempty), never by an exit code; all undertrap … EXIT INT TERMwith absolute paths.HEAD blob for
packages/plugin-detail/src/emptiness.ts=91e9fdde8ae7672e40e9ceef51bd3a007dc0bddf.f119dc261ecd97b279e349051b1513300066acfahasCellValuereturnsfalsefor everything (strictly worse than the bug)b5230f00db5c68029dec3be447c4ad1ecf6212faArray.isArray(value)alone, so every array is EMPTY (the over-correction)7dbf02ed52b73b8a00fb1d396d102eb0ee305fa8A reddens:
AFFORDANCE,COPY AFFORDANCE,COUNTER,⭐ AMPLIFICATION,DECLARED COST,MEASUREMENT.B reddens all of those except
⭐ AMPLIFICATION, plusNON-REGRESSION — a POPULATED array is still a value, and so is a populated object,NON-REGRESSION — 0 and false are still values, andTHE BOUNDARY — {} is a VALUE.C reddens exactly
NON-REGRESSION — a POPULATED array …andMEASUREMENT.THE BOUNDARY. C is the second caricature — "every array is empty" — and only the populated-array axis refuses it.RelatedListis untouched — and the direction is the pointgit difftouches three paths, none of themRelatedList.tsx:RelatedList.isValueEmpty(RelatedList.tsx:913-917) spells(Array.isArray(v) && v.length === 0)and has for some time. The two predicates now agree on every probe measured —[]empty;{},[1],{ a: 1 },0and aDateall values — because the shared authority moved toward the local one. PR #8476 measured that the local shape is the better-shaped answer for a grid and declined to delegate precisely because of this hole; that measurement stands, and nothing here asks it to change.mergeable_state: behindat the time of writing), so its pin fileRelatedList.emptinessAgreement-8459.test.tsxis not on this base. Nothing in this branch depends on it; the agreement above is asserted againstRelatedList.tsxas it stands onc90395b20, and the two branches touch no common file.Tests
Run from the repo root, paths relative to the root, nothing after
--.pnpm exec vitest run packages/plugin-detail/→ 140 files / 1270 tests passedpnpm exec vitest run packages/plugin-detail/src/__tests__/DetailSection.emptyArray-8474.test.tsx→ 9/9, and 9/9 again after the ablation harness restored the treepnpm --filter @object-ui/plugin-detail run type-check→ exit 0 (tsc --noEmit && tsc -p tsconfig.test.json)pnpm --filter @object-ui/plugin-detail run lint→ exit 0,✖ 905 problems (0 errors, 905 warnings); the new test file contributes nonepnpm --workspace-concurrency=2 --filter '@object-ui/plugin-detail^...' build→ exit 0pnpm run check:control-bytes→ exit 0, plus a direct scan of the three changed paths with a lit control that fired on an injected escape-spelled U+0001pnpm run check:unreferenced-sources→ exit 0Declared narrowing.
app-shell,console,plugin-kanban,plugin-calendar,plugin-ganttandexamples/console-starterdepend on@object-ui/plugin-detail, so they are affected packages.hasCellValueis not exported outside the package, and no test outside it reads theNo valueaffordance (grep forNo value/noValueacrossapp-shell,consoleand the three sibling plugins matches six FILES — a CHANGELOG, two metadata-admin inspectors, an i18n table,paramValueShape.tsandresolveActionParams.test.ts— none of them a detail-page render test). I ran five app-shell record-detail files anyway —RecordDetailView.actionUser,RecordDetailView.activityMapIdentity-5878,RecordDetailView.approvalDeclaredActions,auditHistoryDisplay,deriveRelatedLists.relatedListFilter-4664→ 5 files / 61 tests passed. The rest of those packages is left to CI; this narrowing is declared, not silently skipped.Changeset
.changeset/8474-empty-array-is-not-a-cell-value.md,@object-ui/plugin-detail: minor— the landed precedent for aplugin-detailrender change (objectui#8457, and PR #8476 for the same surface). The gate's own verdict line:node scripts/check-changeset-no-major.mjs→✅ No changeset declares a "major" bump.majoris forbidden in this repo (one fixed group).skip-changesetwas not applied — it is a phantom label here, and this change is user-visible anyway.node scripts/check-governed-queue-guard.mjs --test THE-THREE-CHANGED-PATHS(passed explicitly, never bare) →✅ NOT GOVERNED — 3 path(s) checked against 5 governed surface(s); none matched.Out-of-scope finding, filed unassigned
objectui#8481 —
SelectCellRenderer's own opening guard isvalue == null || value === '', which[]passes, so the renderer itself draws a childless flex-wrap DIV. This card and PR #8476 both guard it with an upstream pre-check inside@object-ui/plugin-detail; the shared renderer in@object-ui/fieldsis still wrong for every consumer that does not.ObjectGridis one, verified: it calls the resolved renderer with the raw value at five sites and its onlyEmptyValuefallback is the no-renderer default path, whose guard has the same hole.Notes for review
d7fecfbcf), soemptiness.tsis a real file onmainand no PR-ref reading was needed. PR fix(plugin-detail): RelatedList's column pruning and its cell placeholder share ONE definition of emptiness #8476, by contrast, is still open, so its pin file is not available to lean on.issue_writecreate (objectui#8481) they came back intact through a read-back to the end of the body. One observation is not a refutation — the rule may describe an intermittent or endpoint-specific behaviour — so I kept the mitigation everywhere anyway, since spelling shapes out in words costs nothing. Recording it because a rule nobody can reproduce eventually gets ignored for the wrong reason.packages/specis not involved; this is a renderer-side emptiness decision, not a metadata contract, so contract-first does not apply.🤖 Generated with Claude Code
https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S
Generated by Claude Code